Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose peer information to handlers and clients #364

Merged
merged 2 commits into from
Sep 23, 2022
Merged

Expose peer information to handlers and clients #364

merged 2 commits into from
Sep 23, 2022

Conversation

akshayjshah
Copy link
Member

On requests and streams, expose the peer's address. For clients, the address is the host or host:port, parsed from the URL. For servers, the address is an IP:port pair, provided by net/http as Request.RemoteAddr.

In https://github.com/bufbuild/connect-go/issues/357, a handful of users asked for this information for logging, per-IP rate limiting, and a variety of other server-side concerns. It's also necessary for OpenTelemetry interceptors (https://github.com/bufbuild/connect-go/issues/344).

Fixes https://github.com/bufbuild/connect-go/issues/357.

Where the generated code replaces Request with a stream, expose the
Spec.
On requests and streams, expose the peer's address. For clients, the
address is the host or host:port, parsed from the URL. For servers, the
address is an IP:port pair, provided by `net/http` as
`Request.RemoteAddr`.

In #357, a handful of users asked for this information for logging,
per-IP rate limiting, and a variety of other server-side concerns. It's
also necessary for OpenTelemetry interceptors (#344).

Fixes #357.
@akshayjshah
Copy link
Member Author

@elliotmjackson and @joshcarp, added you as reviewers in case you're interested or want to use this as a chance to start digging into connect-go. No pressure if you're still getting ramped up on other stuff.

@akshayjshah akshayjshah added the enhancement New feature or request label Sep 22, 2022
@@ -217,6 +220,10 @@ type connectClient struct {
protocolClientParams
}

func (c *connectClient) Peer() Peer {
return newPeerFromURL(c.URL)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to be the URL passed to the client constructor. Do you think people would want to have this be set to the URL which was ultimately accessed (in case of redirects)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly? I'm not sure how to accomplish that, though, since we need this information before we've attempted to make a request.

// contains the host or host:port from the server's URL. When accessed
// server-side, Addr contains the client's address in IP:port format.
type Peer struct {
Addr string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gRPC exposes this as net.Addr (which includes a Network string: https://pkg.go.dev/google.golang.org/grpc/peer#Peer). Do you think we won't need to provide that additional data?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly? We can always expose a Network attribute down the road too. I'm not sure how to reliably detect non-TCP transports through the net/http abstractions - for example, unix domain sockets are sort of the wild west, and I have no idea what HTTP/3 will end up looking like.

@akshayjshah akshayjshah merged commit 062122c into main Sep 23, 2022
@akshayjshah akshayjshah deleted the ajs/peer branch September 23, 2022 17:43
akshayjshah added a commit that referenced this pull request Jul 26, 2023
* Add Spec to some user-facing streams

Where the generated code replaces Request with a stream, expose the
Spec.

* Expose peer information to servers and clients

On requests and streams, expose the peer's address. For clients, the
address is the host or host:port, parsed from the URL. For servers, the
address is an IP:port pair, provided by `net/http` as
`Request.RemoteAddr`.

In #357, a handful of users asked for this information for logging,
per-IP rate limiting, and a variety of other server-side concerns. It's
also necessary for OpenTelemetry interceptors (#344).

Fixes #357.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible to retrieve peer info?
2 participants